Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

e2e: fix e2e test for the sriovnetworknodepolicy #742

Conversation

tobiasgiese
Copy link
Contributor

@tobiasgiese tobiasgiese commented Jul 19, 2024

The current Nvidia e2e tests are failing. To resolve this issue, we need to verify only the specified VF range. Without this change, we are mistakenly expecting VFs to have certain values that have not been synchronized by the controller.

We have to verify only the VF range we have specified.
If not we will expect VFs to have certain values that have not been synced by the controller.
Copy link

Thanks for your PR,
To run vendors CIs, Maintainers can use one of:

  • /test-all: To run all tests for all vendors.
  • /test-e2e-all: To run all E2E tests for all vendors.
  • /test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs, Maintainers can use one of:

  • /skip-all: To skip all tests for all vendors.
  • /skip-e2e-all: To skip all E2E tests for all vendors.
  • /skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
    Best regards.

@tobiasgiese
Copy link
Contributor Author

/test-e2e-nvidia-all

@coveralls
Copy link

Pull Request Test Coverage Report for Build 10010164013

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.03%) to 43.073%

Files with Coverage Reduction New Missed Lines %
controllers/drain_controller.go 1 68.06%
Totals Coverage Status
Change from base Build 9977659385: 0.03%
Covered Lines: 6265
Relevant Lines: 14545

💛 - Coveralls

Copy link
Member

@zeeke zeeke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tobiasgiese
Copy link
Contributor Author

Nvidia e2e test CI failed.
Tested manually on the CI jobs node, should work as expected.

/test-e2e-nvidia-all

@ykulazhenkov ykulazhenkov merged commit 18c4695 into k8snetworkplumbingwg:master Jul 22, 2024
14 checks passed
@tobiasgiese tobiasgiese deleted the fix-sriovoperatornodepolicy-test branch July 22, 2024 06:36
@SchSeba
Copy link
Collaborator

SchSeba commented Jul 22, 2024

Hi @tobiasgiese any plans to run the conformance tests on NVIDIA CI? that suite contain much more tests and validations

@tobiasgiese
Copy link
Contributor Author

Hi @tobiasgiese any plans to run the conformance tests on NVIDIA CI? that suite contain much more tests and validations

You mean the tests from /test-nvidia-e2e-all? They run successfully already

@SchSeba
Copy link
Collaborator

SchSeba commented Jul 23, 2024

@tobiasgiese
Copy link
Contributor Author

tobiasgiese commented Jul 23, 2024

@SchSeba the conformance tests will run with the k8s job, see

- name: run test
run: make test-e2e-conformance-virtual-k8s-cluster-ci

the task run test here: https://github.com/k8snetworkplumbingwg/sriov-network-operator/actions/runs/10010164013/job/27671036378

@SchSeba
Copy link
Collaborator

SchSeba commented Jul 23, 2024

yep I know I create that job :P

the problem is that it runs on virtual hardware (qemu virtual IGB driver) that is why I wanted to ask if NVIDIA can run that on MLX cards it will help us cover the mlx plugin code in CI

@SchSeba
Copy link
Collaborator

SchSeba commented Jul 24, 2024

cc @tobiasgiese

@tobiasgiese
Copy link
Contributor Author

the problem is that it runs on virtual hardware (qemu virtual IGB driver) that is why I wanted to ask if NVIDIA can run that on MLX cards it will help us cover the mlx plugin code in CI

Okay, now I got you :)
I like the idea! Let me discuss this with the team for sure, I'll keep you posted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants